-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Backport] Fix for #16069: Configurable product price is not displayed if all children are out of stock and even if Display Out of Stock Products is set to "yes" #18776 #18739
Conversation
…ldren are out of stock and even if Display Out of Stock Products is set to "yes" - allow "out of stock" items to be used for price determination of configurable product ONLY in case all child products are "out of stock".
…ldren are out of stock and even if Display Out of Stock Products is set to "yes" - add strict types.
@novikor - please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a blank line (comment is left near needed piece of code).
...de/Magento/ConfigurableProduct/Model/ResourceModel/Product/Type/Configurable/StockStatus.php
Show resolved
Hide resolved
…ldren are out of stock and even if Display Out of Stock Products is set to "yes" - refactor variable names.
…ldren are out of stock and even if Display Out of Stock Products is set to "yes" - add blank space.
*/ | ||
protected function isApplySalableCheck(SaleableInterface $salableItem): bool | ||
{ | ||
return (bool)$salableItem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object will be always converted to true
, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I’ve missed it. This method from Catalog module should always return true.
Will change it tomorrow.
In general this method was added to make it possible to skip salable check in child classes without need to rewrite _toHtml method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orlangur - I've removed unneeded usage of the variable ( check this https://github.com/magento/magento2/pull/18739/files/f4ec67e5aada173a27586131598d5b24957472d6#diff-5f0a1c702f3ad19b38e301107e2852daR97 ). However, there was a need to add @SuppressWarnings(PHPMD.UnusedFormalParameter)
phpdoc for isApplySalableCheck
method to make it pass static tests.
Goal of this method isApplySalableCheck
: make price rendering depending not only on isSalable
check but also on addaitional check that can be modified in child classes without need to rewrite _toHtml method.
I've used this "newly added" posibility in this class Magento\ConfigurableProduct\Pricing\Render\FinalPriceBox
An other way to achieve this goal is to rewrite _toHtml method that will require a big amount of copy&paste code due to private
properties and methods usage.
So I do prefer currently implemented solution.
…ldren are out of stock and even if Display Out of Stock Products is set to "yes" - remove unneeded usage of the variable.
Hi @vpodorozh. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
…ldren are out of stock and even if Display Out of Stock Products is set to "yes"
@magento-engcom-team give me test instance |
Hi @vpodorozh. Thank you for your request. I'm working on Magento instance for you |
Hi @vpodorozh, here is your new Magento instance. |
@orlangur - could you please review my PR? |
Hi @vpodorozh. The branch |
Closing as corresponding 2.3 PR is closed. |
Hi @vpodorozh, thank you for your contribution! |
Original Pull Request
#18776
Description (#16069)
Configurable product price is not displayed if all children are out of stock and even if Display Out of Stock Products is set to "yes"
Fixed Issues
Preconditions
Manual testing scenarios
Expected result
Price for configurable product should be displayed
Actual result
Price is not displayed at all
Contribution checklist